Skip to content

Sync project assets from the standardized "template" versions #270

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Sep 2, 2021
Merged

Sync project assets from the standardized "template" versions #270

merged 9 commits into from
Sep 2, 2021

Conversation

per1234
Copy link
Contributor

@per1234 per1234 commented Sep 2, 2021

We have assembled a collection of reusable project assets:
https://github.com/arduino/tooling-project-assets
These are used in the repositories of all Arduino tooling projects.

Some minor fixes, improvements, and standardizations have been made in the upstream "template" assets, and those are introduced to this repository via this pull request.

The `ignore-words-list` and `skip` settings of the codespell configuration file may required project-specific adjustments
to fix false positives or avoid positives from externally maintained files. The other settings are universal. It will be
convenient to have the settings the user might need to adjust at the place of highest visibility in the configuration
file.
The `Exclude` array in the `.ecrc` configuration file contains the regular expressions of paths that should be excluded
from checking by the editorconfig-checker tool.

Previously, the "template" file contained a list of filenames that should always be ignored. Although it did work, it was
not really correct because the `.` in the filename is actually a regex wildcard, which is not what was intended. Although
a false match was unlikely, this also might mislead users adding project-specific exclusions to the file regarding the
nature of the exclude patterns. Changing to very explicit patterns avoids any chance of false matches and also makes it
clear that these are regexes.
The `.gitmodules` file defines the properties of a repository's submodules. The file automatically generated by Git
submodule commands use tabs for indentation.

It uses the same file format as the Git configuration file (e.g., `.gitconfig`). Even though the `.gitconfig` file is not
likely to be found under the repository tree, it's possible the `.editorconfig` might end up being used outside the
project specific scope so I added it to the file pattern.
Some complex and lengthy commands are used to repackage the macOS build of the Go application after notarization. Adding
some escaped line breaks and indenting the line continuations to indicate the command structure makes it easier to read
and understand.

This was already done for the other complex commands of the workflows, but was neglected for the repackaging step.
The default behavior of the `actions/checkout` action is to do a shallow fetch of the repository, which is the most
efficient if all that's needed is a copy of the repository files. In cases where the repository history is needed, this
behavior is not appropriate, and so it can be configured via the `fetch-depth` input. Setting this input to 0 causes the
full history to be fetched.

A full fetch is required for workflows that use the `arduino/create-changelog` action to generate a raw changelog from
the commit history, and so it is found in the "Release" workflow. However, there is no use of the commit history by the
"Publish Tester Build" workflow, which does not need a changelog.

The unnecessary full fetch makes the workflow less efficient and more difficult to understand, so it must be removed.
The `create-release` job of the "Release" workflow creates a GitHub release and uploads the built binaries as release
assets. Since the binaries come from the workflow artifact, there is no need for this job to checkout the repository. The
pre-release identification step uses the `GITHUB_REF` environment variable defined by GitHub Actions, which is not
dependent on the runner having a repository checked out.

This means the checkout step serves no purpose. Since it makes the workflow less efficient and more difficult to
maintain, it must be removed.
The subfolders that remain in the Go release system's build output cause a bunch of warnings in the workflow run summary
and logs resulting from the step that uploads the archives as release assets:

```
Warning: Artifact is a directory:dist/serial-discovery_0.0.42_Linux_32bit/. Directories can not be uploaded to a release.
Warning: Artifact is a directory:dist/serial-discovery_0.0.42_Linux_64bit/. Directories can not be uploaded to a release.
Warning: Artifact is a directory:dist/serial-discovery_0.0.42_Linux_ARM64/. Directories can not be uploaded to a release.
Warning: Artifact is a directory:dist/serial-discovery_0.0.42_Linux_ARMv6/. Directories can not be uploaded to a release.
Warning: Artifact is a directory:dist/serial-discovery_0.0.42_Linux_ARMv7/. Directories can not be uploaded to a release.
Warning: Artifact is a directory:dist/serial-discovery_0.0.42_macOS_64bit/. Directories can not be uploaded to a release.
Warning: Artifact is a directory:dist/serial-discovery_0.0.42_Windows_32bit/. Directories can not be uploaded to a release.
Warning: Artifact is a directory:dist/serial-discovery_0.0.42_Windows_64bit/. Directories can not be uploaded to a release.
```

There is no problem because these subfolders are not intended to be added as release assets and the archives in the root
of the dist folder are uploaded, but I think these warnings might cause someone confusion far in the future when nobody
remembers exactly how these workflows work and wonders if these warnings indicate something is wrong.

I didn't find a clean solution for adjusting the artifacts input glob to exclude the subfolders so I settled on adding an
explanatory comment to the workflow.
The `go:build` task has an `LDFLAGS` taskfile template variable, which is currently used by some projects to inject the
versioning information while building via an `-ldflags` flag.

`go test` has undocumented support for an `-ldflags` flag as well. Projects might find it useful to be able to set the
value of string variables in the code while running the tests. To provide support by the task for this use case, a
`TEST_LDFLAGS` taskfile template variable is added to the `go test` command in the `go:test` task, as well as empty
definition of the variable. Even though this project doesn't currently have any need for the capability, it does no harm
and brings the task into alignment with the standardized "template".
The tasks under the `deps` key run in parallel, which is not appropriate for the task that is intended to format the
output from the documentation generation tasks. The generation tasks can run concurrently, so they stay in `deps`, but
the formatting task must be placed in `cmds`, which will cause it to run after all tasks in `deps` have finished.
@per1234 per1234 added type: bug type: enhancement Proposed improvement topic: infrastructure Related to project infrastructure labels Sep 2, 2021
@codecov-commenter
Copy link

codecov-commenter commented Sep 2, 2021

Codecov Report

Merging #270 (0ba7657) into main (4e6c832) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #270   +/-   ##
=======================================
  Coverage   89.67%   89.67%           
=======================================
  Files          44       44           
  Lines        6496     6496           
=======================================
  Hits         5825     5825           
  Misses        549      549           
  Partials      122      122           
Flag Coverage Δ
unit 89.67% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.


Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f04d7ea...0ba7657. Read the comment docs.

@per1234 per1234 merged commit 882b13f into arduino:main Sep 2, 2021
@per1234 per1234 deleted the sync-assets branch September 2, 2021 08:13
@rsora rsora added the type: imperfection Perceived defect in any part of project label Sep 22, 2021
@per1234 per1234 self-assigned this Nov 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: infrastructure Related to project infrastructure type: enhancement Proposed improvement type: imperfection Perceived defect in any part of project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants